swift test: Fix incorrect wasmkit arguments#9114
Conversation
`wasmkit` expects executable arguments to be passed after `--` delimiter. Additionally, we need to bridge current directory to the host with the additional `--dir .` option. Fixes #9113
MaxDesiatov
added
bug
cross-compilation
swift test
MaxDesiatov
requested review from
dschaefer2,
jakepetroules and
shawnhyam
as code owners
MaxDesiatov
requested review from
bkhouri,
cmcgee1024,
daveyc123 and
owenv
as code owners
|
@swift-ci test |
| args.append(runnerPath.pathString) | ||
| args.append(contentsOf: runner.extraCLIOptions) | ||
| args.append(testPath.relative(to: localFileSystem.currentWorkingDirectory!).pathString) | ||
| if runner.path?.components.last == "wasmkit" { |
There was a problem hiding this comment.
Do we have a more robust way to check for WasmKit?
There was a problem hiding this comment.
I don't think so. Yes, it feels somewhat icky to check for a path component, but test runners from toolsets and Swift SDKs provide no other identification. Given that's all we have, why wouldn't we consider this simple and robust enough?
|
@swift-ci test |
|
No new tests were added, as impacted |
|
@swift-ci test windows |
I've addressed that in the comment above, currently no new tests can be added, because none of the existing |
Can we add a test similar to the changes in #9085, more specifically where we inspect the command line arguments passed to the test runner? |
This requires no new tests, as mentioned above this was addressed for the existing test in 6023552 commit in this same PR.
No, this was discussed in of the older PRs that added relevant modifications to |
| args.append(contentsOf: runner.extraCLIOptions) | ||
| args.append(testPath.relative(to: localFileSystem.currentWorkingDirectory!).pathString) | ||
| if runner.path?.components.last == "wasmkit" { | ||
| args.append("--") |
There was a problem hiding this comment.
wasmkit expects executable arguments to be passed after -- delimiter.
That has been fixed by swiftwasm/WasmKit#175, and the fix has been shipped as 0.1.4. So if you're going to merge swiftlang/swift#80820, you don't need to add the -- delimiter.
There was a problem hiding this comment.
That's a great point, thanks!
|
@swift-ci test macos self hosted |
|
@swift-ci test linux |
Enabling this check requires swiftlang/swift-package-manager#9114 to be merged first, which is currently failing due to an unfixed regression on `main`.
After swiftlang/swift#80820 was merged this change is no longer needed.
MaxDesiatov
dismissed
bkhouri’s stale review
Change that wasn't covered with a test was reverted, as it is no longer needed after swiftlang/swift#80820
|
@swift-ci test |
|
@swift-ci test windows |
|
@swift-ci test windows self hosted |
|
@swift-ci test self hosted windows |
6 participants
wasmkitexpects executable arguments to be passed after--delimiter. Additionally, we need to bridge current directory to the host with the additional--dir .option.Fixed #9113
rdar://160218251